fix!: stop deleting $CARGO_HOME during uninstall by Cloud0310 · Pull Request #4861 · rust-lang/rustup

Cloud0310 · 2026-05-18T07:21:32Z

fix #285
This PR fix the cargo bin nuke and the path removing possilble toutoc issue.
Here's the implementation details:

compare cargo bin dir content to rustup binary, if it's a link, remove it
then, we remove the rustup binary, on success, we then remove the cargo bin from path

This prevents rustup self uninstall from nuking out the $CARGO_HOME/bin and possible toctou problem of removing up the path.

Cloud0310 · 2026-05-19T13:42:17Z

I think I may need more tests against this PR, this includes:

$CARGO_HOME/bin with unrelated files
empty bin cleanup test

Should there be more tests? Please let me know @FranciscoTGouveia @rami3l .

Cloud0310 · 2026-05-20T18:20:13Z

I've add more tests about the PR here. Please review.

rami3l

LGTM modulo some tiny nits, nice work :D

View changes since this review

Avoid removing $CARGO_HOME when running uninstall cleanup. Combine all follow-up work for PR rust-lang#4861: - keep $CARGO_HOME/bin when it is non-empty - cover cargo bin cleanup behavior in uninstall tests - update uninstall cleanup comments and related docs - rename cleanup internals for clearer intent

djc

I think this PR needs a bunch of work, it seems to have a lot of changes together in a single commit that are pretty messy. It would be good if the commit history made it clear that there are a sequence of smaller changes being made.

View changes since this review

Avoid removing $CARGO_HOME when running uninstall cleanup. Combine all follow-up work for PR rust-lang#4861: - keep $CARGO_HOME/bin when it is non-empty - cover cargo bin cleanup behavior in uninstall tests - update uninstall cleanup comments and related docs - rename cleanup internals for clearer intent

rustbot · 2026-06-01T12:50:48Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

rami3l · 2026-06-01T14:37:09Z

@Cloud0310 Since your PR needs more work, I've marked this PR as draft.

Please address or at least respond to all open threads before marking this PR as "ready for review" again, at which point the team will return for another round of review, thanks 🙏

rami3l

This is looking much better than before! I have a few final nits; address them in addition to the remaining ones and we should be good to go 🙏

View changes since this review

rami3l · 2026-06-07T09:29:38Z

        utils::remove_dir("rustup_home", &rustup_dir)?;
    }

+    // Clean up rustup tool links


Nit: Logically this snippet should be placed after the "for dirent in diriter {" block, since the removal of cargo home should start after that.

…odule

On Windows, rustup cannot delete the running rustup.exe directly. The uninstall path spawns rustup-gc-* and lets that child finish cleanup after the parent process exits. PATH cleanup now depends on whether the gc process successfully removes /home/cloud/.cargo/bin. That means the parent's --no-modify-path choice has to be passed to the gc process. Use a private environment variable for that one-way handoff because this is an internal child process, not a public CLI invocation.

Cloud0310 force-pushed the main branch from 64f3d84 to 1e3a7f7 Compare May 18, 2026 12:11

FranciscoTGouveia reviewed May 19, 2026

View reviewed changes

Comment thread tests/suite/cli_self_upd.rs

Cloud0310 marked this pull request as ready for review May 19, 2026 10:37

Cloud0310 force-pushed the main branch from 07d31c8 to b6195c7 Compare May 19, 2026 13:22

rami3l reviewed May 19, 2026

View reviewed changes

Comment thread src/cli/self_update.rs Outdated

Cloud0310 requested a review from FranciscoTGouveia May 20, 2026 03:55

Cloud0310 force-pushed the main branch from b6195c7 to 48acf08 Compare May 20, 2026 06:54

djc reviewed May 20, 2026

View reviewed changes

Comment thread src/cli/self_update/windows.rs Outdated

Comment thread src/cli/self_update/windows.rs Outdated

Comment thread src/cli/self_update/windows.rs Outdated

Comment thread src/cli/self_update.rs Outdated

Comment thread src/cli/self_update.rs Outdated

Cloud0310 force-pushed the main branch from 48acf08 to b509a72 Compare May 20, 2026 14:03

Cloud0310 force-pushed the main branch from 5a91816 to 905c540 Compare May 20, 2026 18:33

rami3l reviewed May 21, 2026

View reviewed changes

Comment thread src/cli/self_update/unix.rs Outdated

rami3l approved these changes May 21, 2026

View reviewed changes

Comment thread src/cli/self_update.rs Outdated

Comment thread tests/suite/cli_self_upd.rs Outdated

Comment thread tests/suite/cli_self_upd.rs

Cloud0310 force-pushed the main branch 2 times, most recently from 4a4880a to 971dffa Compare May 22, 2026 08:13

Cloud0310 force-pushed the main branch from 971dffa to b7bddd9 Compare May 22, 2026 08:26

Cloud0310 force-pushed the main branch from b7bddd9 to 01cfd07 Compare May 22, 2026 08:45

This comment has been minimized.

Sign in to view

Cloud0310 force-pushed the main branch from 01cfd07 to 61b3e49 Compare May 22, 2026 09:00

djc reviewed May 22, 2026

View reviewed changes

Comment thread src/cli/self_update/unix.rs Outdated

Comment thread src/cli/self_update/windows.rs Outdated

Comment thread src/cli/self_update/windows.rs Outdated

Comment thread src/cli/self_update/windows.rs

Cloud0310 mentioned this pull request May 22, 2026

Update contribute docs with cross platform code development guide #4864

Closed

rami3l mentioned this pull request May 22, 2026

Clarify in the dev guide what is expected from a PR #4865

Closed

Cloud0310 force-pushed the main branch from 61b3e49 to a80abc9 Compare May 22, 2026 16:28

Cloud0310 force-pushed the main branch 3 times, most recently from afd4a48 to 6e27966 Compare June 1, 2026 13:54

rami3l marked this pull request as draft June 1, 2026 14:17

Cloud0310 force-pushed the main branch 7 times, most recently from 2acb9ec to 1c41abe Compare June 5, 2026 16:09

Cloud0310 added 2 commits June 6, 2026 22:28

refactor: rename delete_rustup_and_cargo_home to clean_cargo_bin

754b48d

refactor: move PATH cleanup into clean_cargo_bin

03caa05

Cloud0310 force-pushed the main branch 2 times, most recently from 781981d to 197f611 Compare June 6, 2026 17:04

rami3l approved these changes Jun 7, 2026

View reviewed changes

Cloud0310 added 2 commits June 7, 2026 17:35

refactor(cli/self-update): move unix::clean_cargo_bin() to parent m…

bb732ae

…odule

doc: add uninstall process overall docstring

2c94106

Cloud0310 force-pushed the main branch from 197f611 to 8991252 Compare June 7, 2026 09:38

fix: remove rustup proxies during uninstall

691bee7

Cloud0310 force-pushed the main branch 2 times, most recently from fd7441d to dac53d8 Compare June 7, 2026 15:17

Cloud0310 added 5 commits June 7, 2026 23:43

fix: preserve CARGO_HOME during uninstall

e67279d

test: cover cargo bin removal during uninstall

75f2bc0

test: cover cargo bin PATH cleanup during uninstall

f3c41ca

test: cover Windows cargo bin cleanup during uninstall

3c0d7e4

Cloud0310 force-pushed the main branch from dac53d8 to 3c0d7e4 Compare June 7, 2026 15:43

Conversation

Cloud0310 commented May 18, 2026

Uh oh!

Uh oh!

Cloud0310 commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Cloud0310 commented May 20, 2026

Uh oh!

Uh oh!

rami3l left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

djc left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rustbot commented Jun 1, 2026

Uh oh!

rami3l commented Jun 1, 2026

Uh oh!

rami3l left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rami3l Jun 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Cloud0310 commented May 19, 2026 •

edited

Loading

rami3l left a comment •

edited by rustbot

Loading

djc left a comment •

edited by rustbot

Loading

rami3l left a comment •

edited by rustbot

Loading

rami3l Jun 7, 2026 •

edited

Loading